Fix AndroidMessageHandler response body cancellation#11554
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
/review |
|
✅ Android PR Reviewer completed successfully! |
There was a problem hiding this comment.
Pull request overview
This PR fixes delayed cancellation observation when AndroidMessageHandler is blocked reading the response body (after headers are received). It does so by wrapping the response stream to actively disconnect/close the underlying HttpURLConnection when the read cancellation token is canceled, and adds regression coverage for both ResponseContentRead and ResponseHeadersRead scenarios (body read canceled after headers).
Changes:
- Add a cancellation-aware response body stream wrapper that aborts stalled reads by calling
HttpURLConnection.Disconnect()and disposing the response stream when cancellation is requested. - Wire the wrapped stream into
AndroidMessageHandlerresponse content creation. - Add
HttpListener-based device tests covering prompt body-read cancellation for both completion options.
Show a summary per file
| File | Description |
|---|---|
src/Mono.Android/Xamarin.Android.Net/AndroidMessageHandler.cs |
Wraps response content stream to make body reads cancellation-aware by aborting the underlying Java connection/stream. |
tests/Mono.Android-Tests/Mono.Android-Tests/Xamarin.Android.Net/AndroidMessageHandlerCancellationTests.cs |
Adds new tests verifying response body cancellation is observed promptly for ResponseContentRead and ResponseHeadersRead. |
tests/Mono.Android-Tests/Mono.Android-Tests/Mono.Android.NET-Tests.csproj |
Includes the new cancellation test file in the test project build. |
Copilot's findings
- Files reviewed: 3/3 changed files
- Comments generated: 1
There was a problem hiding this comment.
⚠️ Needs Changes
Summary: This PR adds cancellation support for response body reads in AndroidMessageHandler by wrapping the response stream in a CancellationAwareResponseStream that disconnects the HttpURLConnection when a CancellationToken is canceled. The approach is sound — registering a cancellation callback to force-close the connection is the right pattern for unblocking stalled Java I/O.
Issues found:
| Severity | Count | Category |
|---|---|---|
| 2 | Resource management, Thread safety | |
| 💡 suggestion | 2 | Testing |
Key concern: The CancellationAwareResponseStream takes dispose ownership of httpConnection, but AndroidHttpResponseMessage also disposes the same connection instance — creating double-dispose and unclear ownership. The AbortRead callback and Dispose method also race on stream.Dispose().
Positive callouts:
- Clean separation of the cancellation wrapper as a nested class
- Good use of
CancellationToken.Register+ exception filter pattern - The
ShouldMapToCancellationexception mapping covers the right exception types from the Java interop layer - Well-structured test server with proper synchronization via
TaskCompletionSource - Tests cover both
ResponseContentReadandResponseHeadersReadcompletion options
Generated by Android PR Reviewer for issue #11554 · ● 14.1M
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
/review |
|
✅ Android PR Reviewer completed successfully! |
There was a problem hiding this comment.
✅ LGTM — Clean, well-designed fix
Summary: The PR wraps the response stream in a CancellationAwareResponseStream that registers a cancellation callback to disconnect the HttpURLConnection and dispose the stream, unblocking stalled reads. The approach is sound — it correctly handles the ReadAsync/CopyToAsync paths that StreamContent actually uses, maps the resulting exceptions to OperationCanceledException, and uses Interlocked for thread-safe double-dispose protection.
Positive callouts:
- The
QueueAbortRead→Task.Run(AbortRead)pattern avoids blocking the cancellation callback thread — good attention to detail TaskCreationOptions.RunContinuationsAsynchronouslyon theTaskCompletionSourceprevents deadlocks in the test server- The test infrastructure (
StalledResponseServer) is well-structured with proper cleanup inTearDown - Good coverage of both
ResponseContentReadandResponseHeadersReadcompletion options
CI: dotnet-android ✅ passed. Xamarin.Android-PR not yet visible (expected for a recently pushed direct PR).
Issues: 0 ❌ · 0
Generated by Android PR Reviewer for issue #11554 · ● 12.5M
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
This comment was marked as resolved.
This comment was marked as resolved.
Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com>
|
|
@copilot fix the build error:
|
Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com>
Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com>
Fixed in I removed the duplicate |
|
/review |
|
✅ Android PR Reviewer completed successfully! |
There was a problem hiding this comment.
⚠️ Needs Changes
Nice fix for response body cancellation — the CancellationAwareResponseStream approach with Disconnect() + stream disposal is a clean way to unblock stalled reads. The test design with a stalled HTTP server is well thought out.
Issues found
| Severity | Count |
|---|---|
| ❌ Error | 1 |
| 💡 Suggestion | 2 |
❌ Missing field declaration — stalledResponseServer is used but never declared in the test class. This will fail to compile.
💡 Per-read CancellationToken.Register — The registration/disposal happens on every ReadAsync call in a tight loop. Consider registering once and disposing in Dispose.
💡 Test coverage — Consider a positive test (non-canceled read completes normally) to verify the wrapper doesn't break the happy path.
CI Status
- ✅
dotnet-android— succeeded - ✅
license/cla— succeeded - ⏳
Xamarin.Android-PR— not yet reported (will likely surface the compile error above)
Generated by Android PR Reviewer for issue #11554 · ● 17.4M
|
@copilot address code reviews |
Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com>
Addressed in |

Fixes #11548
Summary
AndroidMessageHandlerHttpURLConnectionand disposing the response stream when the read token is canceledHttpListener-based coverage forResponseContentReadandResponseHeadersReadbody cancellationTesting
make allANDROID_SERIAL=R58Y30HZ65V ./dotnet-local.sh build -t:RunTestApp tests/Mono.Android-Tests/Mono.Android-Tests/Mono.Android.NET-Tests.csproj -p:IncludeCategories=AndroidMessageHandlerCancellation